@W-18669904- PDP changes for pickup in store feature#2537
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
packages/template-retail-react-app/app/components/product-view/index.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.jsx
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.jsx
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.jsx
Outdated
Show resolved
Hide resolved
…angesForPickupInStore Signed-off-by: snilakandan13 <119348013+snilakandan13@users.noreply.github.com>
…tps://github.com/SalesforceCommerceCloud/pwa-kit into t/commerce/W-18669904/pdpChangesForPickupInStore
packages/template-retail-react-app/app/hooks/use-add-to-cart-modal.js
Outdated
Show resolved
Hide resolved
packages/template-retail-react-app/app/components/product-view/index.jsx
Show resolved
Hide resolved
patricksullivansf
left a comment
There was a problem hiding this comment.
Great discussion. Approving for merge to feature branch with understanding discussion items will be followed up with additional WI based on TLR feedback.
|
The follow up PR will be W-18751492 for PDP page changes |
|
How do I test drive this? |
| - Improved the layout of product tiles in product scroll and product list [#2446](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2446) | ||
| - Updated 6 new languagues [#2495](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2495) | ||
| - Show Bonus Product Label on OrderSummary component [#2524](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/2524) | ||
| - Added support for Shop in Store Functionality |
There was a problem hiding this comment.
that is just the group heading, the PR link is in line #7, the shopInStore is going to have multiple Prs, the first one is the one to the PDP page
| useTheme, | ||
| Stack, | ||
| Radio, | ||
| RadioGroup |
There was a problem hiding this comment.
Let's keep the hook at the bottom, and arrange the components in alphabetical order for readability
| const [pickupError, setPickupError] = useState('') | ||
| const {site} = useMultiSite() | ||
|
|
||
| // Get store info from localStorage for stock status display (move to main render scope) |
There was a problem hiding this comment.
There are a lot of logic is added into this component. Can we create a hook or components for the entire "delivery" section ?
There was a problem hiding this comment.
This whole section is going to be modified based on the changes to the hook for "useDerivedProduct" which Yuming is going on currently, this is more of a placeHolder until then
With that the "useDerivedProduct" will return the storeStockStatus
There was a problem hiding this comment.
Again all this goes to the feature branch, with the caveat that future WIs/stories incrementally are present to encapsulate code better
| } catch (e) { | ||
| return null | ||
| } | ||
| }) |
There was a problem hiding this comment.
This is not a good React practise to listen to a value change.
If you want to create listener for any value change, you should useEffect and put your logic there.
Using useState is not a way to listen for a change. useState is a way to trigger a re-rendering by changing the state.
useEffect(() => {
// update logic here
const val = JSON.parse(window.localStorage.getItem(storeInfoKey))?.inventoryId
setSelectedInventoryId(val)
}, [site.?id])
There was a problem hiding this comment.
I have used useState to set the initial value, for listening to the storechanges I have used "useEffect" in line 85 below
Testing the PDP page after using the store locator , and then verifying using the radio buttons, I have added the screenshots |
| // The add item endpoint in the shopper baskets API does not respect variant selections | ||
| // for bundle children, so we have to make a follow up call to update the basket | ||
| // with the chosen variant selections |
There was a problem hiding this comment.
QQ: i missed this discussion. why are all these unrelated comments removed?
There was a problem hiding this comment.
Please add back this comment since this comment explains why we are doing the following code
There was a problem hiding this comment.
Will do, I'm addressing these with my next PR
|
|
||
| // since the returned data includes all products in basket | ||
| // here we compare list of productIds in bundleProductItems of each productItem to filter out the | ||
| // current bundle that was last added into cart |
@W-18669904
Description
-If the Pickup in Store radio button is selected, the AddtoCart API call will have the inventoryId set for the particular store and the ProductItem
Types of Changes
Changes
How to Test-Drive This PR
Checklists
General
Accessibility Compliance
You must check off all items in one of the follow two lists:
or...
Localization